Skip to content

Change ActiveEntityModifier and add further checks to DraftCancelAttachmentsHandler #669

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Merged
merged 6 commits into from
Nov 14, 2025
Merged

Change ActiveEntityModifier and add further checks to DraftCancelAttachmentsHandler #669

merged 6 commits into from
Nov 14, 2025

Conversation

@lisajulia
Copy link
Contributor

@lisajulia lisajulia commented Nov 6, 2025 •
edited
Loading

This PR fixes: #526
Changes:

  • I have changed the ActiveEntityModifier that is used in the DraftCancelAttachmentsHandler to construct SELECT statements for attachment-related entities.
    The problem was: when "fiori": { "draft_messages": true }, the UI sends refs with multiple segments, this caused problems before with the SELECT statement construction, now the SELECT statement is constructed differently. I have then renamed the ActiveEntityModifier to ModifierToCreateFlatCQN, to reflect better what it does.
  • I've added the checks (isAttachmentEntity(context.getTarget()) || hasAttachmentAssociations(context.getTarget())) to the method DraftCancelAttachmentsHandler:processBeforeDraftCancel, such that the content of this method is only executed when we cancel sth. attachment-related.
  • I've added a comment to the method DraftCancelAttachmentsHandler:isWhereEmpty; this function verifies that we are really cancelling a draft and not doing sth else.

@lisajulia lisajulia mentioned this pull request Nov 6, 2025
Open
@lisajulia lisajulia marked this pull request as draft November 6, 2025 06:56
@lisajulia lisajulia force-pushed the fix-deletion-error branch 4 times, most recently from 16e0372 to e8d63f6 Compare November 7, 2025 16:16
@lisajulia lisajulia changed the title Add check 'skipBeforeDraftCancel' to processBeforeDraftCancel method,… Change ActiveEntityModifier to ActiveEntityModifierFlattener Nov 7, 2025
@lisajulia lisajulia requested a review from beckermarc November 7, 2025 16:17
@lisajulia lisajulia marked this pull request as ready for review November 7, 2025 16:18
@lisajulia lisajulia marked this pull request as draft November 7, 2025 17:12
@lisajulia lisajulia force-pushed the fix-deletion-error branch 3 times, most recently from 45a73b0 to 70f98c4 Compare November 11, 2025 15:49
@lisajulia lisajulia changed the title Change ActiveEntityModifier to ActiveEntityModifierFlattener Add EntityFlattener to DraftCancelAttachmentsHandler Nov 11, 2025
@lisajulia lisajulia changed the title Add EntityFlattener to DraftCancelAttachmentsHandler Add EntityFlattener and further checks to DraftCancelAttachmentsHandler Nov 11, 2025
@lisajulia lisajulia marked this pull request as ready for review November 11, 2025 15:58
@lisajulia lisajulia marked this pull request as draft November 11, 2025 15:59
@lisajulia
Copy link
Contributor Author

lisajulia commented Nov 11, 2025

@rashmiangadi11: I will leave this in draft until Mengmeng has tested this.

@lisajulia lisajulia changed the title Add EntityFlattener and further checks to DraftCancelAttachmentsHandler Change ActiveEntityModifier and add further checks to DraftCancelAttachmentsHandler Nov 12, 2025
@lisajulia lisajulia force-pushed the fix-deletion-error branch 2 times, most recently from 96a0457 to 8a38210 Compare November 12, 2025 07:17
@lisajulia lisajulia force-pushed the fix-deletion-error branch 2 times, most recently from c14f771 to 1502553 Compare November 12, 2025 10:21
@lisajulia
Add 'isAttachmentEntity' and 'hasAttachmentAssociations' checks to
2982ac0 
DraftCancelAttachmentsHandler
@lisajulia
Change ActiveEntityModifier such that it flattens the delete requests
dfee963 
Then they can be correctly transformed into select statements in the
DraftCancelAttachmentsHandler
@lisajulia
Add test to RegistrationTest.java so we reach the required jacoco cov…
e19621e 
…erage again
@lisajulia lisajulia marked this pull request as ready for review November 12, 2025 12:53
@lisajulia
Copy link
Contributor Author

lisajulia commented Nov 12, 2025

Update from @rashmiangadi11: Apparently this fixes Mengmengs problem 👍 , thus I've marked this PR as ready for review.

@lisajulia lisajulia mentioned this pull request Nov 12, 2025
Open
@lisajulia lisajulia requested a review from beckermarc November 14, 2025 08:21
rashmiangadi11
rashmiangadi11 approved these changes Nov 14, 2025
View reviewed changes
Copy link
Collaborator

@rashmiangadi11 rashmiangadi11 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@lisajulia lisajulia merged commit b7c9994 into main Nov 14, 2025
4 checks passed
@lisajulia lisajulia deleted the fix-deletion-error branch November 14, 2025 09:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@adamandre79 adamandre79 adamandre79 approved these changes

@beckermarc beckermarc Awaiting requested review from beckermarc

+1 more reviewer

@rashmiangadi11 rashmiangadi11 rashmiangadi11 approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Deleting attachment does not work for certain "fiori": {"draft_messages": true / false } setting

5 participants

@lisajulia @beckermarc @rashmiangadi11 @adamandre79